Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Peform single push on upload in case of multiple releases #314

Merged
merged 2 commits into from
Oct 29, 2024

Conversation

amalgamm
Copy link
Contributor

Resolves the problem mentioned in this PR

Lets say you want to upload more than one release and place it near the index file. After the first push your second push will be rejected with an error "Updates were rejected because a pushed branch tip is behind its remote". So either you need to pull changes before push another release or push all releases once in the end of upload action.

In case of pulling changes and pushing every release separately you'll face github will launch as many "pages build and deployment" actions as there are new releases you want to upload. All except one will be canceled, but the list of actions will look like a mess.

So the most efficient way is to make single push in the end of upload.

@amalgamm amalgamm force-pushed the single_push_on_upload branch from 8157b4c to 5fd540c Compare August 1, 2023 16:53
@amalgamm
Copy link
Contributor Author

amalgamm commented Aug 7, 2023

@davidkarlsen can you have a look at this pr?

@gaelgatelement
Copy link

I just realized that my PR is a duplicate of this one, and this one is better as it fixes the tests. #336

Can we have someone take a look at this PR please ?

@cpanato cpanato closed this Oct 27, 2023
@cpanato cpanato reopened this Oct 27, 2023
@cpanato
Copy link
Member

cpanato commented Oct 27, 2023

does anyone can provide some manual tests on this? @amalgamm @gaelgatelement and show the logs and maybe the ci job

@gaelgatelement
Copy link

I'm using this fork in our own CI runs : https://github.com/vector-im/ess-starter-edition-core/actions/runs/6624187779

@cpanato
Copy link
Member

cpanato commented Oct 27, 2023

thanks @gaelgatelement
@amalgamm can you rebase and fix the lint issues?

@cpanato
Copy link
Member

cpanato commented Oct 27, 2023

tested on my side and seems to work fine :)

@ibice
Copy link

ibice commented Nov 23, 2023

Hi! I just run into the issue this PR fixes, it'd be cool to have it merged!

Signed-off-by: Ivan Panteleev <ispanteleev@gmail.com>
Signed-off-by: amalgamm <ispanteleev@gmail.com>
Signed-off-by: amalgamm <ispanteleev@gmail.com>
@amalgamm amalgamm force-pushed the single_push_on_upload branch from 85d7dad to 8ea3170 Compare February 5, 2024 11:37
@amalgamm
Copy link
Contributor Author

amalgamm commented Feb 5, 2024

@cpanato I think I fixed the lint. Sorry it took so long

@cpanato
Copy link
Member

cpanato commented Feb 22, 2024

Did you test the case with PackagesWithIndex set as well? not sure if will change any behavior

@amalgamm
Copy link
Contributor Author

Did you test the case with PackagesWithIndex set as well? not sure if will change any behavior

I didn't but I think this case is covered by tests

@jbrandli
Copy link

please merge

@marquesj2-ppb
Copy link

marquesj2-ppb commented May 9, 2024

Any way to get this merged? @cpanato

@0xThresh
Copy link

0xThresh commented Jun 5, 2024

Sorry to tag again @cpanato but I'm running into the described issue as well and am hoping to see this merged. Thank you!

@cpanato
Copy link
Member

cpanato commented Jul 2, 2024

I did a test using the latest chart-release and two chart updates and i see one push: https://github.com/cpanato/testing-ci-providers/actions/runs/9764682879/job/26953586398

Is this PR trying to update as well? If so, are we good, or am I missing something else?

cc @davidkarlsen

@KrisJohnstone
Copy link

Bump.

@stephaneetje
Copy link

stephaneetje commented Oct 9, 2024

Hello, does this PR have any chance to get merged anytime ??

@OddKMS
Copy link

OddKMS commented Oct 10, 2024

Adding my voice to this - would really like to see this get merged so that our Private GH Page Helm Chart "repo" can function properly with multiple charts.

Copy link
Member

@cpanato cpanato left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks!

@cpanato cpanato merged commit f7e4366 into helm:main Oct 29, 2024
4 checks passed
@cpanato
Copy link
Member

cpanato commented Oct 29, 2024

will be good if someone uses the chart-releaser from main and confirm this is working

@Justin-DynamicD
Copy link

is there a prerelease tag or similar we can use to download? I don't see a build associated with main at this time.

@cpanato
Copy link
Member

cpanato commented Nov 7, 2024

is there a prerelease tag or similar we can use to download? I don't see a build associated with main at this time.

we dont build and push main branch, but if that is the community want we can setup that
for now you can clone and build yourself

@Justin-DynamicD
Copy link

Justin-DynamicD commented Nov 7, 2024

is there a prerelease tag or similar we can use to download? I don't see a build associated with main at this time.

we dont build and push main branch, but if that is the community want we can setup that for now you can clone and build yourself

It's more of an ease of testing ask. Without a prerelease tag I have to clone, build, put into a custom container, then modify my existing pipelines to no longer download but instead leverage the installed version. In contrast I can just change the tag and test it if it were pre-tagged.

I'm assuming you branch per release so you can cherry pick and hotfix then? I honestly didn't look at your branching strategy.

At anyrate Ill build locally and give it a test, just figured id take the shortcut if it was available

@Justin-DynamicD
Copy link

Quick update:

quick tests are promising, but most my pipelines use chart releaser action which has no way to use an existing version of cr (checking the code it always downloads). I would have to replace the action with a custom run step which then obfuscates testing (is cr having a problem or do I suck writing my own GHA steps?)

If you are capable of creating a pre-release version (i see its been done in the past) I can give it a more "real world" test, otherwise I can only do simple mocks.

@mijailr
Copy link

mijailr commented Dec 2, 2024

This is something I need as well. Has there been any progress on this?

@Justin-DynamicD
Copy link

I would love to have some better insight on the release process in general. From what I can tell, this has been merged sense October, but going on 2 months later we still don`t have a release. What is the process for this and how can I help it along?

@cpanato
Copy link
Member

cpanato commented Dec 18, 2024

@Justin-DynamicD needs more time as this is on my free time, also if you want to help, please review the PRs, triage issues, and submit any improvements that you might want to see

release is out: https://github.com/helm/chart-releaser/releases/tag/v1.7.0

will update the action in a few days

@Justin-DynamicD
Copy link

thank you very much for the update!

I was going to report that I have been running off the main branch all week without issue so the merge seems solid, but Ill happily switch to 1.7.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.